- 
                Notifications
    You must be signed in to change notification settings 
- Fork 837
Add fallback logic to thanos promql engine #6630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fallback logic to thanos promql engine #6630
Conversation
dc78802    to
    8af8936      
    Compare
  
    8af8936    to
    bbc0cc1      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work
| if qf.thanosEngine != nil { | ||
| res, err := qf.thanosEngine.MakeInstantQuery(ctx, q, fromPromQLOpts(opts), qs, ts) | ||
| if err != nil { | ||
| if engine.IsUnimplemented(err) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a counter metric for number of fallbacks because new engine doesn't support it?
Similar to
https://github.com/thanos-io/promql-engine/pull/518/files#diff-2e6c4934f63ff9b712c2c346b33036af4724adf70b0801fff9b74f71b37fcd89L180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add a metric and update the pr.
1dcc56d    to
    c37ece7      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a unit test at least?
c37ece7    to
    3f2a618      
    Compare
  
    Signed-off-by: SungJin1212 <[email protected]>
3f2a618    to
    8762531      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
This PR adds a fallback logic to the thanos-promql engine.
FYI. The current version of the thanos-promql supports fallback logic, so there is no user impact.
Which issue(s) this PR fixes:
Fixes #6624
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]